-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
libindex: add fallback path for O_TMPFILE not being supported #1292
Conversation
e493183
to
ff437ae
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1292 +/- ##
==========================================
+ Coverage 55.82% 55.87% +0.04%
==========================================
Files 265 266 +1
Lines 16597 16627 +30
==========================================
+ Hits 9266 9290 +24
- Misses 6367 6374 +7
+ Partials 964 963 -1 ☔ View full report in Codecov by Sentry. |
899da53
to
13cb984
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, just a couple of clarifying comments
15d0212
to
313f0ed
Compare
// On OSX, temporary files are not unlinked from the filesystem upon creation, | ||
// because an equivalent to Linux's "/proc/self/fd" doesn't seem to exist. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @hdonnay, do you mind elaborating on this or rephrasing the comment? I'm having a hard time grokking it.
... temporary files are not unlinked from the filesystem upon creation,
because an equivalent to Linux's "/proc/self/fd" doesn't seem to exist.
I couldn't find a reason "/proc/self/fd"
was required for "unlinking" temporary files. I'm also not sure what unlinking in this context means; my assumption based on the word choice would be that the implementation does some sort of symlinking and we need to unlink them at some point, but I don't think that's the case.
temporary files are not unlinked from the filesystem upon creation
I'm particularly confused by this. If we're on Linux, do other temporary files get unlinked when we create our temporary files?
fwiw, I'm not sure if others do this, but I know Ross and I both run the ACS Scanner on macOS devices, so sorting this out would help us out a lot (in other words, I'm not trying to be pedantic with this comment 🙂)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(also happy to move this discussion to another PR or elsewhere if we want to unblock merging this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries.
"Unlink" in this context is a filesystem/syscall term-of-art: it means making a name no longer visible. This isn't exactly deleting a file because the semantics change based on the the type of object, the underlying filesystem, and outstanding file descriptions. Out of a combination of battle scars and habit, I tend to say "unlink" when I'm specifically thinking of unlink(2)
.
With regards to /proc/self/fd
, that's a directory containing magic symlinks (actual terminology, see symlink(7)
) representing open file descriptors. Opening one of them creates a new file descriptor backed by a new file description (see the NOTES section of open(2)
). This means that unlike dup(2)
, the uses of the two file descriptors are completely independent (e.g. seeking one will not change the offset on the other).
The Linux code uses this to open files that aren't linked into the filesystem (meaning they'll never outlast the process, because only the process has them open) but can still be reopened as many times as needed to create independent file descriptors.
AFAICT, there's not a way to do this on OSX. Looking at FreeBSD man pages, fdescfs(5)
acts the same way but only if mounted with nodup
, which I don't think is the default. There is mention on that manpage about a O_EMPTY_PATH
to openat(2)
, but I can't find any information on whether that exists on OSX and don't have a machine to test with. To work around this, the code simply doesn't unlink the file and re-opens it by name as needed. The downside is that the files are only removed as long as the program shuts down cleanly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the detailed explanation! I need to dive deeper into all this info, so please feel free to consider this discussion non-blocking for the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM passing baton to @BradLugo
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Signed-off-by: Hank Donnay <hdonnay@redhat.com>
will merge with green CI |
/fast-forward |
No description provided.